Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update protos #647

Merged
merged 3 commits into from
Dec 4, 2023
Merged

Update protos #647

merged 3 commits into from
Dec 4, 2023

Conversation

dandavison
Copy link
Contributor

@dandavison dandavison commented Nov 26, 2023

Update protos (needed for Typescript Update work).

This PR sets the new sdk_name and sdk_version fields to their zero values. See #588

@dandavison
Copy link
Contributor Author

Linter violations are fixed in #651

@dandavison dandavison marked this pull request as ready for review December 4, 2023 12:22
@dandavison dandavison requested a review from a team as a code owner December 4, 2023 12:22
Comment on lines +143 to +144
sdk_name: "".to_string(),
sdk_version: "".to_string(),
Copy link
Member

@cretz cretz Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to set these to client_name and client_version from the client if they are different from the previous task (or remain empty/unset if they are the same).

The PR description says "See temporalio/features#321" but this doesn't solve #588 completely. Is this just a temporary thing until #588 can be properly addressed?

Copy link
Contributor Author

@dandavison dandavison Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right. There's a little bit more work and testing required for that because we need to be watching for changes from task-to-task, rather than sending the same value repeatedly. So that's #588. This PR is just for unblocking Typescript work so I was thinking that it's fine to merge this as-is since these changes will result in transmitting the protobuf default zero-values, which is what the server is already using.

@dandavison dandavison merged commit b1e5e4b into temporalio:master Dec 4, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants